feat(gitea): implement GetTaskURI for remote task resolution#2732
feat(gitea): implement GetTaskURI for remote task resolution#2732theakshaypant wants to merge 1 commit into
Conversation
Enable Gitea/Forgejo provider to resolve remote taskRef URLs using the provider's authenticated API instead of returning "not supported". Supports branch, tag, and commit SHA URL formats. Signed-off-by: Akshay Pant <akpant@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the GetTaskURI method for the Gitea provider, enabling the retrieval of task files from Gitea and Forgejo URLs. It introduces a splitGiteaURL helper function to extract organization, repository, reference, and path information, supported by new unit tests. Review feedback highlights the need for more robust URL handling by using EscapedPath and PathUnescape to correctly process encoded characters like slashes and plus signs. Additionally, it is recommended to verify the Gitea client's initialization to prevent potential nil pointer panics.
| uriPath := pURL.Path | ||
| if pURL.RawPath != "" { | ||
| uriPath = pURL.RawPath | ||
| } |
There was a problem hiding this comment.
Using pURL.Path for splitting is problematic because it is already unescaped by url.Parse. If a path segment (such as a branch name or a directory) contains an encoded slash (%2F), pURL.Path will contain a literal /, causing strings.Split to break the segment incorrectly. Using pURL.EscapedPath() ensures you split only on the intended delimiters.
| uriPath := pURL.Path | |
| if pURL.RawPath != "" { | |
| uriPath = pURL.RawPath | |
| } | |
| uriPath := pURL.EscapedPath() |
| if spRef, err = url.QueryUnescape(spRef); err != nil { | ||
| return "", "", "", "", fmt.Errorf("cannot decode ref: %w", err) | ||
| } | ||
| if spPath, err = url.QueryUnescape(spPath); err != nil { | ||
| return "", "", "", "", fmt.Errorf("cannot decode path: %w", err) | ||
| } | ||
| if spOrg, err = url.QueryUnescape(spOrg); err != nil { | ||
| return "", "", "", "", fmt.Errorf("cannot decode org: %w", err) | ||
| } | ||
| if spRepo, err = url.QueryUnescape(spRepo); err != nil { | ||
| return "", "", "", "", fmt.Errorf("cannot decode repo: %w", err) | ||
| } |
There was a problem hiding this comment.
When decoding path segments, url.PathUnescape should be used instead of url.QueryUnescape. QueryUnescape incorrectly treats + as a space, which is not the case for URL path segments. Additionally, since the path is now extracted using EscapedPath(), all segments must be explicitly unescaped to restore their original values.
| if spRef, err = url.QueryUnescape(spRef); err != nil { | |
| return "", "", "", "", fmt.Errorf("cannot decode ref: %w", err) | |
| } | |
| if spPath, err = url.QueryUnescape(spPath); err != nil { | |
| return "", "", "", "", fmt.Errorf("cannot decode path: %w", err) | |
| } | |
| if spOrg, err = url.QueryUnescape(spOrg); err != nil { | |
| return "", "", "", "", fmt.Errorf("cannot decode org: %w", err) | |
| } | |
| if spRepo, err = url.QueryUnescape(spRepo); err != nil { | |
| return "", "", "", "", fmt.Errorf("cannot decode repo: %w", err) | |
| } | |
| if spRef, err = url.PathUnescape(spRef); err != nil { | |
| return "", "", "", "", fmt.Errorf("cannot decode ref: %w", err) | |
| } | |
| if spPath, err = url.PathUnescape(spPath); err != nil { | |
| return "", "", "", "", fmt.Errorf("cannot decode path: %w", err) | |
| } | |
| if spOrg, err = url.PathUnescape(spOrg); err != nil { | |
| return "", "", "", "", fmt.Errorf("cannot decode org: %w", err) | |
| } | |
| if spRepo, err = url.PathUnescape(spRepo); err != nil { | |
| return "", "", "", "", fmt.Errorf("cannot decode repo: %w", err) | |
| } |
| data, _, err := v.Client().GetFile(spOrg, spRepo, spRef, spPath) | ||
| if err != nil { | ||
| return false, "", err | ||
| } | ||
| return true, string(data), nil |
There was a problem hiding this comment.
The v.Client() call returns the underlying Gitea client. If the client has not been initialized, this call will return nil, leading to a panic. According to the repository's rules on lazy initialization, the Client() method should be implemented to return a new instance directly if the internal field is nil, rather than returning nil and requiring the caller to handle the error. This avoids potential panics and ensures thread safety for stateless or lightweight objects.
| data, _, err := v.Client().GetFile(spOrg, spRepo, spRef, spPath) | |
| if err != nil { | |
| return false, "", err | |
| } | |
| return true, string(data), nil | |
| client := v.Client() | |
| if client == nil { | |
| return false, "", fmt.Errorf("no gitea client has been initialized") | |
| } | |
| data, _, err := client.GetFile(spOrg, spRepo, spRef, spPath) | |
| if err != nil { | |
| return false, "", err | |
| } | |
| return true, string(data), nil |
References
- Avoid lazy initialization of stateless and lightweight objects without proper synchronization. Instead, return a new instance directly if the field is nil.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2732 +/- ##
==========================================
+ Coverage 59.36% 59.38% +0.02%
==========================================
Files 208 208
Lines 20626 20678 +52
==========================================
+ Hits 12244 12280 +36
- Misses 7610 7620 +10
- Partials 772 778 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
📝 Description of the Change
Enable Gitea/Forgejo provider to resolve remote taskRef URLs using the provider's authenticated API instead of returning "not supported". Supports branch, tag, and commit SHA URL formats.
🔗 Linked GitHub Issue
N/A
🧪 Testing Strategy
🤖 AI Assistance
AI assistance can be used for various tasks, such as code generation,
documentation, or testing.
Please indicate whether you have used AI assistance
for this PR and provide details if applicable.
Important
Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Claude noreply@anthropic.com
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.